Skip to content

Conversation

@alex-spies
Copy link
Contributor

Relates #121884.

We currently cannot run all queries that have a SORT before either LOOKUP JOIN or MV_EXPAND. This is because we cannot push down these commands past SORTs without breaking the sort order in some cases. We may or may not want to choose to push down in these cases in the future, even though this affects sort order.

Moreover, further work and generalizations of LOOKUP JOIN may be even less compatible with sorting - e.g. if we generalize lookup indices to have multiple shards residing on different nodes.

To keep all possibilities open, let's just document that we give no guarantees whatsoever; we can revisit this later when needed.

@alex-spies alex-spies requested review from costin and leemthompo May 5, 2025 16:41
@elasticsearchmachine elasticsearchmachine added Team:Docs Meta label for docs team Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) labels May 5, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-docs (Team:Docs)

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

::::

::::{warning}
The output rows produced by `LOOKUP JOIN` can be in any order and may not
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leemthompo , should we also place this warning in the more in-depth landing page for lookup join?

Copy link
Contributor

@leemthompo leemthompo May 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are running the risk of those two pages becoming undifferentiated and therefore hard to maintain, if we repeat everything in both. Also adding admonitions like this doesn't scale.

We could consider moving this warning and the tip above it into the landing page under a new heading like Usage notes. And then we can link to that new section of the landing page from here with a message like "Refer to Usage notes for important information about using the LOOKUP JOIN command"— instead of repeating admonitions on both pages. This will be cleaner, more future-proof, and easier to maintain.

WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pushed with c1cc777

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the new structure. Thanks, Liam!


### Handling name collisions

In case of name collisions, the newly created columns will override existing columns.
Copy link
Contributor

@leemthompo leemthompo May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alex-spies @costin this is pre-existing info unchanged in this PR, but was wondering if there was additional guidance about workarounds we could give for dealing with name collisions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, good idea!

Like this?

Such columns can be preserved by using `RENAME` to assign non-conflicting names before the `LOOKUP JOIN`.

And yes, we realize this is clunky and we want to have a more ergonomic approach to avoid this renaming dance.


For a complete list of supported data types and their internal representations, see the [Supported Field Types documentation](/reference/query-languages/esql/limitations.md#_supported_types).

## Implementation details
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, that's not really an implementation detail, that's more the semantics that the command provides.

Maybe "behavior details" or "semantic details" or "results" or something along these lines?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup agreed that was rough placeholder— we'll go with Usage notes for future-proofery :)

@leemthompo leemthompo enabled auto-merge (squash) May 7, 2025 08:49
@leemthompo leemthompo merged commit 9e3ae5b into elastic:main May 7, 2025
6 checks passed
@leemthompo leemthompo added the auto-backport Automatically create backport pull requests when merged label May 7, 2025
leemthompo pushed a commit to leemthompo/elasticsearch that referenced this pull request May 7, 2025
elasticsearchmachine pushed a commit that referenced this pull request May 7, 2025
ywangd pushed a commit to ywangd/elasticsearch that referenced this pull request May 9, 2025
afoucret pushed a commit to afoucret/elasticsearch that referenced this pull request May 9, 2025
jfreden pushed a commit to jfreden/elasticsearch that referenced this pull request May 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL auto-backport Automatically create backport pull requests when merged >docs General docs changes Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:Docs Meta label for docs team v8.18.1 v8.19.0 v9.0.1 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants